Skip to content

Stop reporting transient backend 5xx errors from media location sync to Sentry#266

Merged
nyomanjyotisa merged 1 commit into
mainfrom
claude/fix-media-location-502-sentry-noise-wy
Jun 11, 2026
Merged

Stop reporting transient backend 5xx errors from media location sync to Sentry#266
nyomanjyotisa merged 1 commit into
mainfrom
claude/fix-media-location-502-sentry-noise-wy

Conversation

@sentry

@sentry sentry Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

What

The media location sync (updateMediaLocation) no longer reports ServerError (HTTP status ≥ 500) to Sentry. It now skips reporting for ServerError alongside the existing AbortError exclusion, while still reporting genuine failures such as network errors and unexpected exceptions.

Why

Media location sync is non-critical background tracking that fires periodically while a buyer plays an audio/video file. The previous catch block reported every error to Sentry except AbortError, so transient backend 5xx responses showed up as Sentry issues.

Root cause of the surfaced issue: the Gumroad backend returned 502 Bad Gateway for POST /api/mobile/media_locations. No backend spans appeared in the trace, meaning Rails never processed the requests — the Heroku router returned 502 before reaching the app. Heroku routers return 502 when no healthy dyno is available (dyno crash, restart, or memory pressure). The window was ~50 minutes and affected only 2 users, consistent with a transient period of partial backend unavailability rather than a code bug.

Because the sync still captured all ServerErrors (status ≥ 500), this infrastructure event surfaced as an actionable issue in Sentry even though there is nothing to fix in the app. Transient 5xx responses from the backend are not actionable mobile-side bugs, so they are treated the same way as timeout-driven AbortErrors: logged via console.warn but not sent to Sentry.

useAPIRequest already retries ServerErrors with exponential backoff, so genuine 5xx noise is reduced elsewhere; this change prevents the non-critical fire-and-forget sync from polluting Sentry with infrastructure blips.

Before / After

This is a non-user-facing change (error-reporting behavior only). There is no UI change.

  • Before: A backend 502 during media playback produced Failed to update media location: ServerError: Request failed: 502 in Sentry.
  • After: The same 502 is logged locally via console.warn and not reported to Sentry. Network errors and other unexpected exceptions are still reported.

Test Results

Added two tests to tests/lib/media-location.test.ts:

  • does not report transient ServerError (5xx) to Sentry (502 response from fetch)
  • does not report a directly thrown ServerError to Sentry

Both tests fail when the fix is reverted, confirming they exercise the new behavior. Existing tests (including the reports non-AbortError exceptions to Sentry case) continue to pass, so genuine errors are still reported.

Test Suites: 2 passed, 2 total
Tests:       15 passed, 15 total

npx tsc --noEmit passes with no errors.


🤖 AI disclosure: This PR was written by Claude Opus 4.6. Prompt: fix the GUMROAD-MOBILE-WY issue where the mobile app reported transient backend 502s from media location sync to Sentry, ensure the fix is fully working, add tests, and open a PR.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Note

Low Risk
Observability-only change in a non-critical background sync path; real errors still go to Sentry.

Overview
Media location sync (updateMediaLocation) still logs failures with console.warn and still swallows errors, but Sentry reporting now treats ServerError (HTTP ≥ 500) like AbortError: transient backend 5xx (e.g. Heroku 502) are no longer sent to Sentry.

Genuine failures (e.g. network errors and other non-ServerError exceptions) continue to call Sentry.captureException.

Tests in tests/lib/media-location.test.ts cover both a 502 fetch response and a directly thrown ServerError, while the existing “reports non-AbortError” case still asserts real errors are reported.

Reviewed by Cursor Bugbot for commit 8b7740a. Bugbot is set up for automated code reviews on this repo. Configure here.

…to Sentry

The media location sync is non-critical background tracking, but it
captured every ServerError (status >= 500) to Sentry. Transient backend
infrastructure events such as Heroku 502s (returned by the router when no
healthy dyno is available) are not actionable code bugs, so they surface
as Sentry noise.

Skip Sentry reporting for ServerError alongside the existing AbortError
exclusion. Genuine failures (network errors, unexpected exceptions) are
still reported.

Fixes GUMROAD-MOBILE-WY
@sentry

sentry Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@claude review once

@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR stops updateMediaLocation from forwarding transient backend ServerError (HTTP ≥ 500) exceptions to Sentry, treating them the same as AbortError timeouts: logged via console.warn but not reported. Network errors and unexpected exceptions continue to reach Sentry.

  • lib/media-location.ts — imports ServerError from @/lib/request and adds an instanceof ServerError guard alongside the existing AbortError name-check; the choice of instanceof over a name-check is correct because ServerError is a well-known class in this codebase rather than a platform error type.
  • tests/lib/media-location.test.ts — two new tests cover the 502-via-fetch-response path and the directly-thrown ServerError path, both confirmed to fail when the fix is reverted.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to Sentry reporting behavior for a non-critical fire-and-forget sync, with no effect on user-facing functionality or data integrity.

The implementation correctly uses instanceof ServerError (a known custom class) rather than a name-check, the new guard doesn't accidentally swallow any errors that previously reached Sentry for genuine failures, and both new tests are correctly wired to the real code path — one through a 502 fetch response and one through a directly-thrown ServerError. Existing tests including the 'reports non-AbortError exceptions' case remain unchanged.

No files require special attention.

Important Files Changed

Filename Overview
lib/media-location.ts Adds ServerError import and excludes it from Sentry reporting alongside the existing AbortError exclusion; logic is correct and well-commented.
tests/lib/media-location.test.ts Adds two targeted test cases — one via a 502 fetch response, one via a directly-thrown ServerError — both of which correctly verify the new Sentry exclusion behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[updateMediaLocation called] --> B{accessToken present?}
    B -- No --> C[return early]
    B -- Yes --> D[requestAPI POST /mobile/media_locations]
    D -- Success --> E[done]
    D -- Throws --> F{catch block}
    F --> G[console.warn]
    G --> H{error type?}
    H -- AbortError\ntimeout/poor connectivity --> I[skip Sentry]
    H -- ServerError\ntransient 5xx from backend --> I
    H -- Other error\nnetwork, unexpected --> J[Sentry.captureException]
Loading

Reviews (1): Last reviewed commit: "Stop reporting transient backend 5xx err..." | Re-trigger Greptile

@gumclaw gumclaw self-assigned this Jun 9, 2026
@nyomanjyotisa nyomanjyotisa self-assigned this Jun 11, 2026
@nyomanjyotisa nyomanjyotisa merged commit bca9bf1 into main Jun 11, 2026
2 checks passed
@nyomanjyotisa nyomanjyotisa deleted the claude/fix-media-location-502-sentry-noise-wy branch June 11, 2026 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants